1067 improve abm tests #1141
1067 improve abm tests #1141
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1141 +/- ##
=======================================
Coverage 96.59% 96.59%
=======================================
Files 137 137
Lines 11057 11063 +6
=======================================
+ Hits 10680 10686 +6
Misses 377 377 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
The changes to parameters look fine, only the loop got changed.
The additional comments in and on the test are really nice, especially on the more complicated ones. Though I am unsure whether the doxygen format should be used on a TEST makro over a regular comment.
This definitely improves the tests, but I will keep the linked issue open for now.
| for (auto age_group = AgeGroup(0); age_group < AgeGroup(m_num_groups); ++age_group) { | ||
| for (std::uint32_t variant_count = 0; variant_count < static_cast<std::uint32_t>(VirusVariant::Count); ++variant_count) { | ||
|
|
||
| auto virus_variant = static_cast<mio::abm::VirusVariant>(variant_count); |
There was a problem hiding this comment.
With a patch to make_index_range in index_range.h, this becomes
| for (auto age_group = AgeGroup(0); age_group < AgeGroup(m_num_groups); ++age_group) { | |
| for (std::uint32_t variant_count = 0; variant_count < static_cast<std::uint32_t>(VirusVariant::Count); ++variant_count) { | |
| auto virus_variant = static_cast<mio::abm::VirusVariant>(variant_count); | |
| for (auto age_group : make_index_range(AgeGroup{m_num_groups})) { | |
| for (auto virus_variant : make_index_range(Index<VirusVariant>{VirusVariant::Count})) { |
Try to replace make_index_range with the following, so it can work with AgeGroup (otherwise Index<AgeGroup> is required):
/**
* @brief Construct a range that can be used to iterate over all MultiIndices in the given dimensions.
* The range spans over [0, d) for each category in the MultiIndex, where d is that category's value in dimensions.
* @param[in] dimensions A MultiIndex that contains the dimension for each category.
* @tparam Categories All categories of the given MultiIndex.
* @return An iterable range over the given dimensions.
*/
template <class... Categories>
IndexRange<Index<Categories...>> make_index_range(const Index<Categories...>& dimensions)
{
return IndexRange<Index<Categories...>>(dimensions);
}| // Fuzzing: assign random valid LocationId values and verify correctness. | ||
| for (int i = 0; i < 100; ++i) { | ||
| auto random_id = mio::UniformIntDistribution<int>::get_instance()(rng, 0, 1000); | ||
| person.set_assigned_location(mio::abm::LocationType::Work, mio::abm::LocationId(random_id)); | ||
| EXPECT_EQ(person.get_assigned_location(mio::abm::LocationType::Work), mio::abm::LocationId(random_id)); | ||
| } |
There was a problem hiding this comment.
This can be further improved, for one by making use of the new "RandomNumberTest" (maybe with a random_integer function) to handle the rng and seed logging. And we can fuzz the Location type in addition to the id.
| EXPECT_EQ(person.get_assigned_location(mio::abm::LocationType::Work), mio::abm::LocationId(random_id)); | ||
| } | ||
|
|
||
| // Boundary test cases: test with boundary LocationIds. |
There was a problem hiding this comment.
It says boundary edge cases, but above the upper bound is only 1000, not max. I think the wording here should change to fix this, or maybe do use max() as upper bound for the rng
Changes and Information
Please briefly list the changes (main added features, changed items, or corrected bugs) made:
If need be, add additional information and what the reviewer should look out for in particular:
Benchmark results:
Run on (8 X 24.1214 MHz CPU s)
CPU Caches:
L1 Data 64 KiB (x8)
L1 Instruction 128 KiB (x8)
L2 Unified 4096 KiB (x2)
Load Average: 2.14, 2.46, 2.84
Benchmark Time CPU Iterations
abm_benchmark/abm_benchmark_50k 1300 ms 1294 ms 1
abm_benchmark/abm_benchmark_100k 2599 ms 2586 ms 1
abm_benchmark/abm_benchmark_200k 5220 ms 5187 ms 1
Run on (8 X 24.0948 MHz CPU s)
CPU Caches:
L1 Data 64 KiB (x8)
L1 Instruction 128 KiB (x8)
L2 Unified 4096 KiB (x2)
Load Average: 3.26, 3.54, 3.26
Benchmark Time CPU Iterations
abm_benchmark/abm_benchmark_50k 1311 ms 1306 ms 1
abm_benchmark/abm_benchmark_100k 2611 ms 2607 ms 1
abm_benchmark/abm_benchmark_200k 5242 ms 5225 ms 1
Merge Request - Guideline Checklist
Please check our git workflow. Use the draft feature if the Pull Request is not yet ready to review.
Checks by code author
Checks by code reviewer(s)